perf(coro): compact cancellation callback dispatch state - #1055
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Replaces per-callback mutex/CV synchronization with a compact atomic cancellation state machine while preserving cancellation semantics.
Changes:
- Implements atomic callback selection, dispatch, teardown, and waiting.
- Adds deterministic concurrency/lifetime tests.
- Adds performance benchmarks and documentation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
include/elio/coro/cancel_token.hpp |
Implements atomic callback dispatch synchronization. |
tests/unit/test_cancel_token.cpp |
Expands ordering, lifetime, reentry, and concurrency coverage. |
examples/cancel_callback_benchmark.cpp |
Adds callback performance and allocation benchmarks. |
examples/CMakeLists.txt |
Registers the new benchmark target. |
wiki/Performance-Tuning.md |
Documents callback costs and benchmarking. |
CHANGELOG.md |
Records the optimization and preserved contracts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replace the mutex and condition variable embedded in every cancellation callback node with a native-width atomic dispatch phase and C++20
wait/notifysynchronization.The state machine preserves the current cancellation contract: LIFO dispatch, immediate cancellation, same-dispatch suppression of a selected later callback, non-blocking callback reentry across cancellation sources, external teardown waiting for callback and payload destruction, first-exception propagation after all selected callbacks run, and exactly-once callback-payload destruction.
On GCC 12 x86_64, the base callback node shrinks from 208 to 112 bytes and the task-parent callback node from 224 to 128 bytes. A small-buffer callback's single requested
make_sharedallocation falls from 224 to 128 bytes. Pinned, interleaved Release measurements show improvements across registration, unlink, dispatch, immediate-cancel, and concurrent-unregister paths.Type of Change
Related Issues
Closes #1053
Related to #1048
Changes Made
Core Changes
std::mutex,std::condition_variable, and invoking-thread state with a native-width atomic phase:registered -> claimed -> invoking -> completed, with terminalunregisteredtransitions.claimed; acquire transitions make it visible to dispatch and teardown.claimedcallback tounregistered.claimedorinvoking, and publishcompletedonly after callback invocation and payload destruction.API Changes (if applicable)
No public API, layout, result, callback ordering, or exception-specification change.
cancel_registration::unregister()remainsnoexcept.The layout change is confined to the internal
coro::detail::callback_nodeand its internal task-parent specialization.Migration Guide (if breaking change)
Not applicable.
Testing
Unit Tests
Integration Tests
Sanitizer Testing
Test Results
All builds were out of source with explicit
--parallel 2. Debug and Release benchmark targets build cleanly with warnings as errors.cancel_callback_benchmark --smokeexercises every series; invalid zero, non-numeric, and extra arguments return a usage error.Checklist
Code Quality
Documentation
Testing
Compatibility
CHANGELOG.md(if applicable)Performance (if applicable)
Screenshots / Diagrams
Not applicable.
Additional Notes
The Release comparison used the same final benchmark source and GCC 12 flags for both variants; only the Elio include root differed. It ran 20 interleaved baseline/candidate process pairs. Scalar series were pinned to one CPU; concurrent unregister used two CPUs and rejected samples without both removed and invoked outcomes. Confidence intervals are paired bootstrap intervals over per-process candidate/baseline ratios.
Layout and requested allocation results:
sizeof(callback_node)sizeof(task_parent_callback_node)The allocation figures are requested control-block bytes recorded around
make_shared; they are not allocator usable-size or tcache-class measurements.Selected paired p50 results:
Dispatch p99 ratios were 0.820 for 1 callback, 0.526 for 8, 0.533 for 32, and 0.554 for 256; every paired confidence interval remained on the improvement side.
Concurrent unregister / 32 callbacks improved from 3,476.5 to 2,471.5 ns at p50, 5,187.5 to 4,368 ns at p95, and 5,904 to 5,136 ns at p99. The paired p99 ratio was 0.862 [0.825, 0.943]. All 40 baseline/candidate samples contained both removal and invocation outcomes; aggregate baseline outcomes were 11,627 removed / 308,373 invoked and candidate outcomes were 10,400 / 309,600.
Reviewer Guidance
Areas requiring special attention:
dispatcher_identitythrough theclaimedphase.completed.Questions for reviewers:
Thank you for contributing to Elio! 🎉